fix annotations font color - #2892
Conversation
|
Thanks for this pull request! I'll try to review and merge it soon. The build failure is odd and I've seen it before but it doesn't make a ton of sense. I'll look into it :) |
| zmid = (zmax + zmin) / 2 | ||
| if min_text_color == max_text_color: | ||
| # diverging colorscale | ||
| mid_text_color = "#000000" |
There was a problem hiding this comment.
I'm not sure this is a great default... this is assuming the diverging scale is light in the middle?
There was a problem hiding this comment.
I wonder if we shouldn't accept a third color in font_colors for this and only set this one here if it's not present?
There was a problem hiding this comment.
Yes can be an option but is there any diverging colorscale where the it's not the case?
There was a problem hiding this comment.
Well, users can pass in their own colorscale, they're not limited to just the built-in ones.
| min_text_color, max_text_color = _AnnotatedHeatmap.get_text_color(self) | ||
| z_mid = _AnnotatedHeatmap.get_z_mid(self) | ||
| if zmin is None or zmax is None: | ||
| zmin, zmax = _AnnotatedHeatmap.get_z_min_max(self) |
There was a problem hiding this comment.
here it seems like we overwrite both if either is missing? probably best not to do that I think.
There was a problem hiding this comment.
From what I understand from the doc https://plotly.github.io/plotly.py-docs/generated/plotly.graph_objects.Heatmap.html "zmax – Sets the upper bound of the color domain. Value should have the same units as in z and if set, zmin must be set as well." we can not pass only one of these 2 thats why i overwrite both
There was a problem hiding this comment.
This is true of the underlying heatmap trace, yes, but this figure-factory already does its own bounds-computation, so we may as well reuse it. I've already addressed this comment in #2921.
|
Thanks for this PR! It's prompted me to make this one #2921 which is a bit more targeted and doesn't change any method signatures etc. I'd love your thoughts on the other one, and if we merge that then you can try to build on it to address the color problem in diverging colorscales? |
4 participants
Correct the font annotation color when using zmin and zmax or zmid #2187
